Size Change: +433 B (0%)

Total Size: 1.24 MB

_{compressed-size-action}

I appreciate your PRs. But both Cover and Image (#31366) are very widely used, and this adds a pretty mammoth panel to the inspector for both. I think we need to land a PR to address #31337 before we add these panels.

I appreciate your PRs. But both Cover and Image are very widely used, and this adds a pretty mammoth panel to the inspector for both. I think we need to land a PR to address #31337 before we add these panels.

Thanks for the feedback @jasmussen.

I'm working on a PR for #31337 at the moment though its not yet ready for review.

There are going to be several delays with addressing that one. For example;

• updating border block support to handle separate border radius values
• allowing custom CSS units for width and radius (Block Support: Update border support to allow non-pixel units #31483)
• implementing the new control for linked radius values

Any issues leaving this PR and #31366 open for the time being?

Any issues leaving this PR and #31366 open for the time being?

Not at all! The feature is awesome and we want it. We just want to thread the needle of user experience and features.

I just tested this PR, and it works nicely. Looks good on the backend and frontend.
I look forward to testing the new approach.
Thanks!

@jasmussen This PR has just been rebased to incorporate the recent border support UI changes. Do you feel this is in a place where we can move forward on adding border control to the cover block?

I think it's close, and I think you've done excellent work. Here's what I see:

A few things. If you choose a color, without first choosing a border style, no border is applied:

We should probably either default to the solid border style, or apply it as soon as you set a border width.

In https://github.com/WordPress/gutenberg/pull/31370/files#diff-9e449d4b8bd69c9771de8e20c9a1d78798a6934f99e3bfc432df6ceb23def7f4R14 you apply an overflow: hidden; to enable border radius. While I think border-radius is very compelling, I think we need to find a different way to accomplish it, as overflow: hidden; crops the resize handle:

If I don't choose any color for the border, I'm pretty sure that the border will be the same color as the text, (i.e. currentColor). But it's worth testing in a theme with a black background and white text, just to be sure, and if not, just explicitly set currentColor as the default color.

Question: should we set a border-width of at least 1px if you choose a color before choosing a border width? Just so a border is actually visible if you start there? This is what Figma does:

Another question: should we move the border panel, since it's collapsed by default, to the bottom of the inspector? As noted across a few PRs, I think we want to revisit where border tools live in future iterations. But since it'll launch collapsed by default, it might make sense to group it at the bottom with the other collapsed "Advanced" panel.

Thanks for the thoughtful feedback @jasmussen.

We should probably either default to the solid border style, or apply it as soon as you set a border width.

I'm not convinced we should do either of those just yet but that could be that we aren't quite on the same page with what block supports feature do and provide at this level. Hopefully my understanding of this will be clearer through the responses to your questions below.

Currently, not applying any value in a field provided by a block support feature means the block can inherit values via the stylesheet generated by theme.json or global styles. This is how other block supports such as the padding and margin features inherit values from the theme or global styles. Unfortunately, at this point in time we do not have access to merged theme, global and user styles within the block editor.

As I understand it, the aim of these block support features/styles at the individual block level is to give the user a means by which to override theme or global styles.

Regarding applying a style as soon as a color is selected. It would be possible. The catch there though is not knowing whether or not a style was set via the theme or global styles and this change then overriding that when the user didn't actually ask for it.

I currently have a PR nearly ready to land that will set the border style to none when the user sets the border width to 0. That could be considered a precedent to changing multiple values at once. I see that as a different situation though as a border width of 0 is essentially setting no border and is what the user is asking for.

If I don’t choose any color for the border, I’m pretty sure that the border will be the same color as the text, (i.e. currentColor).

This would depend on the theme. It may set the border color via the border.color style in theme.json or via its CSS.

But it’s worth testing in a theme with a black background and white text, just to be sure, and if not, just explicitly set currentColor as the default color.

This would mean overriding whatever the theme had set already elsewhere. I don't believe we can set defaults that don't match the theme for this reason. Without access to the merged theme/global styles, we could only hope to extract computed styles from DOM elements. Given ad hoc application of block support provided styles for some blocks, accessing computed styles would be hit and miss.

I think generic styles or defaults are better set at the theme.json level.

Question: should we set a border-width of at least 1px if you choose a color before choosing a border width? Just so a border is actually visible if you start there? This is what Figma does:

Automatically changing the width when setting a color would be another case of overriding the theme value when the user might not intend or want to.

Take the example where a theme sets a 3px black border. I just want to change the color to white. In the suggested scenario, I select the white color and it changes the width on me to 1px since the width field was previously blank. The possible inconvenience could be doubled because now as the user without inspecting the block with dev tools I likely don't know what the border width was and need to fiddle with it to get it back consistent with other blocks.

Your Figma example is a little different. In that case, you are adding a new border. Something that doesn't exist yet. So default values make sense, they aren't trying to blindly inherit from a theme or global styles. In the case of border support controls we are attempting to only add overrides to values that might already exist and have values defined elsewhere.

Another question: should we move the border panel, since it’s collapsed by default, to the bottom of the inspector?

All block support provided panels come from the same place in the code. Any change to the order will be reflected across all blocks. In the past, I believe there was discussion around the sidebar and ordering the panels alphabetically.

I'm not opposed to reordering the block support provided panels however that should be a separate PR to this. The impacts any change in order have on all blocks can be discussed on that PR within context. I also don't think it should hold up adding border support to the cover block.

Thanks for the thorough response and sorry for the AFK-belated response.

Regarding the "setting a border style by default" question, my angle on this is purely a user experience one, where it feels broken when setting a width and not seeing any result. So I'm not trying to challenge any approaches to the global styles structure, that seems on a strong path, only how we can handle it from the surface/experience layer.

Regarding applying a style as soon as a color is selected. It would be possible. The catch there though is not knowing whether or not a style was set via the theme or global styles and this change then overriding that when the user didn't actually ask for it.

• Can we provide a default border style, something like border-style: solid; border-width: 0;? With low enough specificity it should make for a default that isn't "nothing".
• Can we chain the UX controls so that setting a width also sets a style?
• Can the width box be grayed until a style is chosen?

I'm sure there are all sorts of challenges with these suggestions, but it can hopefully inspire some better ideas.

This would depend on the theme. It may set the border color via the border.color style in theme.json or via its CSS.
This would mean overriding whatever the theme had set already elsewhere. I don't believe we can set defaults that don't match the theme for this reason. Without access to the merged theme/global styles, we could only hope to extract computed styles from DOM elements. Given ad hoc application of block support provided styles for some blocks, accessing computed styles would be hit and miss.

Right, if the theme sets the color, that should be the color. I'm referring to the cases, which will likely be the majority of cases, where the theme does not set the color, we should fall back to currentColor if at all possible, so that we have good defaults.

To an extent that seems to be what it all boils down to, defaults and baselines:

1. If a theme does not set any specific properties in theme.json, we need a way to set defaults.
2. If a theme sets properties using theme.json, those should be used.
3. If a user sets properties, they need to easily override both 1 and 2.

To elaborate on 1, I mean CSS defaults. Just like how if you do not provide a stylesheet on a webpage, you get Times New Roman in black text on a white background, we need to provide CSS default styles for blocks.

This is where I can see us leveraging :where for providing better defaults than the browser provides. This codepen illustrates how we can provide default colors that are easily overridden by either global styles or users. Essentially:

html :where(.has-red-background-color) {
	background-color: maroon;
}

That sets the default value of the "red" color to maroon.

.has-red-background-color {
	background-color: red;
}

With zero added specificity, the above selector can then update that value to red instead.

It's using this CSS addition to the particular block or blocks or design tool in question, that I'd like to see if we could add defaults. For example:

html :where(.wp-block-cover) {
	border-style: solid;
	border-width: 0;
	border-color: currentColor;
}

☝️ the border-color should already behave that way, i.e. if you specifiy border: 1px solid; it's basically the same as border: 1px solid currentColor;, however if necessary it could still be added. currentColor would then be the same as the color defined on the Cover block.

CSS-tricks has a bit more on using :where to provide baselines: https://css-tricks.com/using-the-specificity-of-where-as-a-css-reset/

Thanks for taking the time to go into such detail @jasmussen, I definitely appreciate it. 👍

I've added a baseline style for a cover block's border.

html :where(.wp-block-cover) {
	border: 0 solid currentColor;
}

As you promised, it provides a means for the user to be more likely to get immediate visual feedback when editing individual border properties.

This doesn't address the issue raised where if a border color is chosen before a width, the user doesn't see a border, unless the theme sets a border width elsewhere by default.

Given we can't easily determine exact defaults or merged border styles and then reflect that within the block support provided controls, I think this is as close as we can get to the optimal experience.

Let me know if there is anything further we need to tweak to see this one merged.

Thanks for trying it. As you note, it's only a small improvement, as you still need a width. And if we do add a >0 width, you get this:

So it seems like the best way experience might be for a border-width to be explicitly set at the same time as selecting a border-style. Which primarily, as I understand it, opens a technical question, so I'd love thoughts on how arduous this would be to accomplish, and then we can go from there. And we might even decide: it's fine — you have to choose a width before you see anything.

One issue we have to solve, though, is the cropped resize handle. Are there any alternatives to applying overflow: hidden; to the container?

So it seems like the best way experience might be for a border-width to be explicitly set at the same time as selecting a border-style. Which primarily, as I understand it, opens a technical question, so I'd love thoughts on how arduous this would be to accomplish, and then we can go from there.

Currently, we do not have access to the merged theme and global styles in the block editor. If we accept that, another possibility I explored was to try and determine the computed border-width style for the block.

Given this needs to be done via block supports and it must handle blocks generically, I don't believe we can correctly access the computed style. Some blocks will skip serialization on the main block wrapper and apply it elsewhere or adjust the value. This means the block support can't determine which element to retrieve computed styles for.

As I see it we are left facing the override problem described earlier.

We can force a border width when a border style is selected. That part is easy. The issue is it comes at the cost of accepting that we are blindly overriding any border width configured by the theme.json or global styles.

If the user wants what was set by the theme or they themselves set in the global styles, they'll need to dig up that value and apply it manually. They'll also then need to update the block any time the theme or global styles changed. This might not be a common use case but it highlights that applying any width value at the block support level is done in absence of knowing what's really being displayed.

Overriding what is set by theme.json and global styles might be acceptable. Hopefully, through this discussion, we're making an informed decision on what taking that position entails.

One issue we have to solve, though, is the cropped resize handle. Are there any alternatives to applying overflow: hidden; to the container?

My apologies. That one had slipped my mind.

After a quick play with the styles, I believe we can apply a border-radius: inherit style on the inner :before element. While testing that however, I found that the introduction of border widths interferes with the resizable box height calculations. That will take me a little more time and testing to sort out but I will address that in the next couple of days.

Thanks for shining the light back on this issue 👍

Hi folks!

I raised the question of what to do about the border styles not appearing until a border style is selected over at #34061 (comment) (setting border control defaults in the tool panel).

The scenario is where we don't show border style by default in the ToolsPanel.

With things the way they are, I think having the styles hidden might exacerbate the UX challenge we have here, namely, folks wondering why the default controls have no effect with no apparent way out (unless they fiddle around with the default controls).

Strictly speaking about the editor, I can empathise with users who might not know that they have to turn on border style to see any border effects.

I see there are concerns in relation to styles overwriting defined higher up the chain however.

Are there any other considerations to bear in mind should we decide to set a default style in the border controls?

Overriding what is set by theme.json and global styles might be acceptable.

Isn't the case that styles set in editor controls such as border will override global styles anyway?

Would we need to communicate that in the editor, or do we expect editors to know this?

Maybe it's a matter of openly communicating this somewhere in the workspace.

It also occurred to me to check the computed styles as a general way of testing existing styles. Not sure how to get around blocks that skip serialization unless we know the target element.

I'm ready to help implement something to mitigate this, but just want to make sure I understand the balancing act correctly 😄

Thank you!

Isn't the case that styles set in editor controls such as border will override global styles anyway?

Yes, adjusting the block support controls deliberately sets whatever the user asks for. The difference here is when adjusting the width the user isn't directly asking for the style to change.

Would we need to communicate that in the editor, or do we expect editors to know this?

My understanding is that we expect users to know if they change something for an individual block that overrides more general settings applied via global styles or theme.json. Again, this is slightly different from them adjusting the width and us making the assumption the style must change as well. We can do it. It would be good to confirm the trade-offs are acceptable first.

Maybe it's a matter of openly communicating this somewhere in the workspace.

I'm not sure how we can communicate this concisely and effectively within the sidebar. It might be worth exploring if there are possibilities where the border width and style controls could be combined, similar to how the "font appearance" control combines both font weight and style e.g. thin italic etc.

It also occurred to me to check the computed styles as a general way of testing existing styles. Not sure how to get around blocks that skip serialization unless we know the target element.

We could attempt to find the computed style of the block itself and use that for all blocks that don't skip the serialization of the block support. It doesn't solve the problem but does help mitigate the UX issue. Combine this with per block styles that apply some default, easily overridden border style via :where or something in CSS and perhaps this edge case becomes manageable.

Can we get a status update on this PR?
Thank you!

Can we get a status update on this PR?

We are currently waiting for the new border controls to land.

There are two PRs adding refined border control components and a third that uses these to improve border support to allow individual side borders. Once they land we will be better positioned to roll out border support across blocks more widely.

• Borders: Add new BorderControl component #37769
• Borders: Add BorderBoxControl component #38876
• Borders: Use new border control components in block support #37770

This PR has been switched back to draft status until some issues within the editor can be worked through.

I've trialled two new approaches to try and align the editor with the frontend when borders are supported on the Cover block.

1. 783bcff was looking good until I realised that in the editor borders overlayed the content whereas on the frontend the content is contained within the border.
2. cba150a is probably the closest I've gotten to aligning editor and frontend. The catch is it wraps the Cover block and a resizable box in a new div. I'm not sure this will be acceptable.

I've left this PR on the second option for the moment and will continue iterating on this next week.

Below demonstrates the current state of the Cover block in this PR.

Approach number 2 didn't work for me in the editor with the default black overlay, but adding the following to the editor.scss fixed it:

.wp-block-cover__background.has-background-dim:not([class*=-background-color]) {
    background-color: #000;
}

I have gone around in circles all day trying various things, but no joy! You can almost use ResizeableBox as the block wrapper div if you change it from self closing to a parent div with rest of block as children, but it breaks a few things as the blockprops ref doesn't link being passed to that functional component.

This is a really frustrating one, as apart from the blue resize outlines being a bit cutoff everything works nicely with a very simple addition of the border supports and an overflow: hidden on the block wrapper. Will keep pondering it.

After some further experimentation I've put up a separate PR exploring an alternate approach to getting the Cover block to behave in the editor with borders. It isn't perfect either unfortunately.

More info in #40774.

In terms of where we are at with adopting border support for Cover blocks, we have some options each with their own problems.

Option 1: Simple opt-in and accept UX issues

Pros

• Simple
• No new markup in editor

Cons

• Drag handles are cut off
• Resizing is jumpy due to height not taking into account border widths (if it was only block specific borders we might be able to overcome this. Once you introduce global styles/theme.json we don't have access to it those values yet)

Option 2: Refactor editor markup to wrap block in new div, make ResizableBox sibling to the block (cba150a)

Pros

• Resizing isn't jumpy
• Drag handles aren't clipped

Cons

• Alters markup in the editor
  • ResizableBox also creates a difference between editor and frontend anyway
  • This PR attempts to minimize it by maintaining most classes and props on the new wrapper (minus the `wp-block-cover)
• Needs some further testing with theme.json/global styles etc

Option 3: Add wrapper within Cover block in editor

Pros

• Probably provides the cleanest UX (ignoring theme.json/global styles)
• No clipping of resize drag handles
• The change in markup has slightly less chance to break things in terms general layout of blocks in the editor (compared with Option 2)

Cons

• The new wrapper needs to be absolutely positioned to avoid interference from padding theme.json or global styles
• Borders applied via theme.json or global styles will not display correctly
  • This is already an issue on some other blocks e.g. Search block see #32417

Option 4: Something else

Open to any ideas on this one 🙏

Option 4: Something else

Another option is to move the ResizableBox that is causing issues into a BlockPopover. A draft PR is up in #41153 though it needs some further work for the placeholder view but is looking promising.

Closing this in favour of bringing #41153 up-to-date.